Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add new medical history options #2190

Closed
wants to merge 30 commits into from
Closed

Add new medical history options #2190

wants to merge 30 commits into from

Conversation

AshrafMd-1
Copy link
Contributor

Proposed Changes

  • Added new medical history options
  • Altered the disease table to include status type & duration

Associated Issue

Architecture changes

  • Remove this section if not used

Merge Checklist

  • Tests added/fixed
  • Update docs in /docs
  • Linting Complete
  • Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

@AshrafMd-1 AshrafMd-1 requested a review from a team as a code owner May 22, 2024 19:16
@AshrafMd-1 AshrafMd-1 mentioned this pull request May 22, 2024
Closed
6 tasks
rithviknishad
rithviknishad requested changes May 23, 2024
View reviewed changes
care/facility/models/patient_base.py Outdated Show resolved Hide resolved
care/facility/models/patient.py Outdated Show resolved Hide resolved
@AshrafMd-1
Copy link
Contributor Author

@rithviknishad also is it required to push the migrations?

@rithviknishad
Copy link
Member

Yup, otherwise the test workflows would fail.

@AshrafMd-1
Copy link
Contributor Author

@rithviknishad can u check once

rithviknishad
rithviknishad reviewed May 24, 2024
View reviewed changes
Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than #2190 (comment) and adding tests for those, LGTM.

@AshrafMd-1
Copy link
Contributor Author

@rithviknishad @sainak
Can you take a look at the validation

rithviknishad
rithviknishad requested changes May 27, 2024
View reviewed changes
Copy link
Member

@rithviknishad rithviknishad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tests needed for the validations

@AshrafMd-1
Copy link
Contributor Author

@rithviknishad where to add tests ?

@rithviknishad
Copy link
Member

@rithviknishad where to add tests ?

https://github.com/coronasafe/care/blob/develop/care/facility/models/tests/test_patient.py#L11

@AshrafMd-1
Copy link
Contributor Author

@rithviknishad are these valid tests
LIke when I run tests it says ok

@AshrafMd-1 AshrafMd-1 requested a review from sainak June 3, 2024 14:55
@nihal467
Copy link
Member

nihal467 commented Jun 4, 2024

@AshrafMd-1 can you fix the lint and test failure

CC: @rithviknishad @AshrafMd-1

@AshrafMd-1
Copy link
Contributor Author

@AshrafMd-1 can you fix the lint and test failure

CC: @rithviknishad @AshrafMd-1

@nihal467 can you check it now

@nihal467
Copy link
Member

LGTM

sainak
sainak requested changes Jul 3, 2024
View reviewed changes
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/api/serializers/patient.py Outdated Show resolved Hide resolved
care/facility/migrations/0439_disease_duration_disease_status_disease_type_and_more.py Outdated Show resolved Hide resolved
@AshrafMd-1
Copy link
Contributor Author

@sainak can you check once

@AshrafMd-1
Copy link
Contributor Author

@rithviknishad the codecov check says it failed

is it required to reach the required percentage ?

@rithviknishad
Copy link
Member

@AshrafMd-1 You can make use of code coverage inline comments to improve the test coverage

image

@nihal467
Copy link
Member

nihal467 commented Aug 8, 2024

@AshrafMd-1 can you look into the comment by rithvik

@AshrafMd-1
Copy link
Contributor Author

@nihal467 can you run the tests once again

CC: @rithviknishad

@nihal467
Copy link
Member

@AshrafMd-1 the tests are failing

@AshrafMd-1
Copy link
Contributor Author

@AshrafMd-1 the tests are failing

I will push a pr by tmr

@AshrafMd-1
Copy link
Contributor Author

@nihal467 i fixed the migration issue can you rerun the tests

@nihal467
Copy link
Member

nihal467 commented Oct 6, 2024

https://rebuildearth.slack.com/archives/C06KGMUAB0V/p1728116762128059?thread_ts=1728075807.176319&cid=C06KGMUAB0V

Closing this PR as the entire medical history will be re-built after verifying proper standards and included in the health details API.

@nihal467 nihal467 closed this Oct 6, 2024
@AshrafMd-1 AshrafMd-1 deleted the medical-history branch October 6, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sainak sainak sainak approved these changes

@rithviknishad rithviknishad rithviknishad approved these changes

@khavinshankar khavinshankar Awaiting requested review from khavinshankar

Assignees
No one assigned
Labels
blocked
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AshrafMd-1 @rithviknishad @nihal467 @khavinshankar @sainak @vigneshhari